Skip to content

feat(mitre): add MITRE ATT&CK mapping with --mitre tactic-grouped output (#5)#66

Merged
Zious11 merged 20 commits intodevelopfrom
feature/mitre-attack-mapping
Apr 13, 2026
Merged

feat(mitre): add MITRE ATT&CK mapping with --mitre tactic-grouped output (#5)#66
Zious11 merged 20 commits intodevelopfrom
feature/mitre-attack-mapping

Conversation

@Zious11
Copy link
Copy Markdown
Owner

@Zious11 Zious11 commented Apr 13, 2026

Closes #5.

Summary

  • New src/mitre.rs module: MitreTactic enum (14 Enterprise + 2 ICS-unique tactics), technique_info(id) -> Option<(name, tactic)> as the single source of truth, plus thin technique_name / technique_tactic projections. 15 pre-seeded technique IDs covering existing emission sites + near-future backlog (Add C2 beaconing detection #3 beaconing, Add Modbus TCP protocol analyzer #7 Modbus, Add DNP3 protocol analyzer #8 DNP3). MitreTactic is #[non_exhaustive] so future MITRE versions can add tactics non-breakingly.
  • TLS analyzer: 3 malformed-SNI findings (AsciiWithControl, NonAsciiUtf8, NonUtf8) now emit mitre_technique = Some("T1027") (Obfuscated Files or Information). Other 4 None sites (weak ciphers, deprecated SSL) intentionally unchanged.
  • New --mitre flag on analyze: regroups the FINDINGS section by MITRE tactic in canonical kill-chain order; sorts within each tactic by verdict-desc → confidence-desc → emission order; expands per-finding MITRE line to T1046 — Network Service Discovery. Unknown IDs render as (unknown) and bucket under "Uncategorized" last. Default behavior (flag off) byte-identical to pre-feature.
  • Coverage sanity test (tests/mitre_tests.rs::known_emitted_technique_ids_resolve_in_lookup) asserts every ID in a hand-curated list of currently-emitted technique IDs resolves via the lookup. The list is hand-maintained — adding a new emission site without updating the list will not fail CI; this is a deliberate trade-off (the hand-curated approach is the idiomatic Rust pattern at this scale per Perplexity validation; see Track canonical-emitted-IDs test is hand-maintained (deliberate trade-off) #67 for the rationale and trigger conditions for revisiting).

Design + validation trail

Spec: docs/superpowers/specs/2026-04-13-mitre-attack-mapping-design.md. Plan: docs/superpowers/plans/2026-04-13-mitre-attack-mapping.md. Every substantive design decision validated against Perplexity (data model, tactic derivation, CLI flag scope, T1027 vs T1036/T1071.001, pre-seeding, error handling, output layout, sort order, MITRE tactic assignments, sub-technique format, ICS-tactic prefix convention, non-deprecation of T0885).

Test plan

  • cargo fmt --all -- --check — clean.
  • cargo clippy --all-targets -- -D warnings — zero warnings.
  • cargo test — 213 passed, 0 failed (was 191 baseline; +22 new tests).
  • Local multi-agent PR review (/pr-review-toolkit:review-pr) iterated to clean across 2 rounds.
  • All deferred review items individually validated against Perplexity before filing.
  • Copilot review iterated across 3 rounds (2 → 4 → 1 inline comments) until clean.

Follow-up issues filed

Zious11 added 17 commits April 13, 2026 00:31
Spec for wiring mitre_technique across all analyzers and adding a
--mitre flag that regroups terminal output by tactic. Option<String>
field stays; new src/mitre.rs module provides technique_name and
technique_tactic lookups backed by exhaustive match statements.
Pre-seeds Enterprise + ICS entries for backlog issues #3, #7, #8.
TLS malformed-SNI findings assigned T1027 (Obfuscated Files or
Information). Every design decision validated against Perplexity.
…ment tactic collision

Validation flagged: (1) Enterprise has 14 tactics canonically; spec was
missing ResourceDevelopment. (2) MITRE convention does not prefix
tactic names with "ICS:" — Caldera/Atomic Red Team/Navigator all use
unprefixed names. (3) Enterprise and ICS share several tactic names
(Discovery, Command and Control, etc.) with different TA-IDs; unified
variants are a documented v1 limitation, splittable later if needed.
All existing tests/*.rs files use _tests.rs suffix; tests/mitre_coverage.rs
broke the pattern. Merging the regression test into tests/mitre_tests.rs
as a #[test] fn keeps naming consistent and reduces file count.
9 tasks, TDD-per-task, small commits. Task 9 is the full CI-equivalent
check (cargo fmt + clippy + test) — encodes the lesson from PR #61
where skipping fmt-check locally caused a CI Format regression on
initial push.
Implements the grouped FINDINGS rendering path in TerminalReporter:
buckets findings by tactic, sorts within each group by verdict then
confidence then original index, emits sections in ATT&CK kill-chain
order, appends Uncategorized last, and expands per-finding MITRE lines
to include the technique name (or '(unknown)' for unrecognised IDs).
…im docs

Local multi-agent review (code, tests, comments, silent failures, type design)
converged on 12 actionable items in 3 buckets — substantive design,
test hygiene, and doc rot. Substantive items validated against
Perplexity before fixing.

Substantive (Perplexity-validated):
- src/mitre.rs: consolidate technique_name + technique_tactic into a
  single source of truth via technique_info(id) -> Option<(name, tactic)>;
  the existing public fns become thin projections. Eliminates the
  parallel-match drift class — adding an ID to one without the other
  is now structurally impossible.
- src/reporter/terminal.rs: extract render_finding_prefix shared by
  flat and grouped per-finding render paths. Common escape/colorize/
  evidence emission lives in one place; each MITRE-line variant owns
  only its trailing two lines.
- tests/mitre_tests.rs: HashSet<MitreTactic> uniqueness check instead
  of format!("{:?}") roundtrip — robust against any future Debug impl
  change.

Test hygiene:
- tests/reporter_tests.rs: anchor tactic-header substring assertions
  on "## " prefix so future analyzer summaries containing the word
  "Discovery" or "Impact" cannot confuse the ordering checks.
- tests/reporter_tests.rs: add test for sort tiebreaker (4 findings
  with identical verdict+confidence must preserve emission order).
- tests/reporter_tests.rs: add test for typo'd-variant separation
  (T1046 known + T1046.999 unknown must end up in distinct buckets).

Doc rot:
- src/mitre.rs: drop "MITRE ATT&CK v18, 14 tactics" version pin;
  drop self-reference "Used by the terminal reporter" from
  all_tactics_in_report_order; drop orphan debug_assert! suggestion
  in technique_name doc (no caller does this and the design dropped
  it during Task 8 review); inline the ICS/Enterprise tactic-name
  collision rationale instead of "see spec / v1 limitation".
- src/reporter/terminal.rs: doc comments on render_finding_prefix,
  render_finding_flat, render_finding_grouped, render_findings_grouped.
- src/analyzer/tls.rs: shared block comment above the SNI match
  explaining why malformed-SNI maps to T1027 (Obfuscated Files or
  Information) rather than T1036 (Masquerading) or T1071.001
  (Web Protocols).
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class MITRE ATT&CK technique/tactic support to findings, including a lookup module and an opt-in terminal output mode that groups findings by tactic via a new --mitre CLI flag.

Changes:

  • Introduces src/mitre.rs (tactics enum + technique ID → name/tactic lookup) and wires it into the crate.
  • Adds --mitre to analyze and updates TerminalReporter to optionally group FINDINGS by tactic and expand technique IDs to ID — Name.
  • Maps TLS malformed-SNI findings to T1027 and adds tests for mapping, grouping/sorting, and CLI parsing.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/mitre.rs New MITRE tactic enum + technique lookup functions (single source of truth).
src/lib.rs Exposes new mitre module.
src/cli.rs Adds --mitre boolean flag to analyze.
src/main.rs Threads --mitre into TerminalReporter as show_mitre_grouping.
src/reporter/terminal.rs Implements grouped rendering path + technique name expansion.
src/analyzer/tls.rs Sets mitre_technique = Some("T1027") for malformed SNI findings.
tests/tls_analyzer_tests.rs Verifies TLS malformed-SNI findings emit T1027.
tests/reporter_tests.rs Adds grouped output behavior tests (ordering, bucketing, sorting, expansion).
tests/mitre_tests.rs Adds tactic/name/tactic-order tests and a “known IDs” regression test.
tests/cli_tests.rs Verifies --mitre parses and defaults false.
docs/superpowers/specs/2026-04-13-mitre-attack-mapping-design.md Design spec for the feature.
docs/superpowers/plans/2026-04-13-mitre-attack-mapping.md Implementation plan / validation trail.
.gitignore Ignores .claude/worktrees/.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/mitre_tests.rs
Comment thread src/reporter/terminal.rs Outdated
Both Copilot comments hit the same theme — the canonical-list test is a
hand-curated sanity check, not an exhaustive cross-check, and the docs
overstated it as the "authoritative typo gate." Renaming the test to
known_emitted_technique_ids_resolve_in_lookup and tightening the doc
comments in tls.rs / terminal.rs accordingly.

The hand-curated approach is preserved (Perplexity-validated as the
idiomatic Rust pattern at this scale per #67); only the wording changes
to match what the test actually does. Filed #67 as the tracked anchor
for the trade-off rationale.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/mitre_tests.rs
Comment thread docs/superpowers/specs/2026-04-13-mitre-attack-mapping-design.md Outdated
Comment thread docs/superpowers/specs/2026-04-13-mitre-attack-mapping-design.md Outdated
Comment thread docs/superpowers/specs/2026-04-13-mitre-attack-mapping-design.md
Three spec sections drifted from reality during brainstorming/planning
and were never reconciled:

- CLI threading section claimed src/dispatcher.rs threads --mitre, but
  that's the stream dispatcher; actual threading is via src/main.rs
  (where Commands::Analyze is destructured). Updated.
- Error-handling section described a debug_assert! at the render site
  that was deliberately dropped during local PR review (Task 8) because
  the grouped-render unknown-ID test would panic under it. Updated to
  document the actual lenient-render behavior.
- Testing-strategy bullet claimed the canonical-IDs test "fails CI if
  an analyzer emits an ID not in the lookup" — overstated; the test is
  hand-curated and non-exhaustive (see issue #67). Updated wording.
- Blast-radius modified-files list referenced src/dispatcher.rs and
  src/reporter/mod.rs and a debug_assert! that aren't in this PR.
  Updated to match the actual touched files.

Implementation unchanged; only the spec is brought in sync.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mitre.rs
Copilot round-3 review revisited the suggestion I had filed as deferred
issue #65: mark MitreTactic #[non_exhaustive] so adding a tactic when
MITRE bumps a new ATT&CK version is non-breaking for downstream crates
that match on the enum.

The original deferral rationale ("no external consumers today") was
weaker than Copilot's argument: the right time to add #[non_exhaustive]
is BEFORE external consumers start exhaustive-matching, because adding
it later would itself become breaking for them. In-crate matches are
unaffected — same-crate matches don't need wildcard arms even with
#[non_exhaustive].

Already Perplexity-validated as the canonical use case ("appropriate
for closed-today, possibly-extended-later enums bound to external
versioned standards") during the deferred-items review round; cited
example was MITRE ATT&CK specifically.

213 tests pass, fmt + clippy clean.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Zious11 Zious11 merged commit 0a00c63 into develop Apr 13, 2026
16 checks passed
@Zious11 Zious11 deleted the feature/mitre-attack-mapping branch April 13, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add MITRE ATT&CK mapping to findings

2 participants